crates/scout/src/main.rs (1)

194-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use structured fields for endpoint logs.

Line 195 and Line 197 interpolate queryable values into messages. Emit address and error as tracing fields instead.

Proposed fix

- tracing::info!("Spawning metrics endpoint on {}", metrics_config.address);
+ tracing::info!(address = %metrics_config.address, "spawning metrics endpoint");
  if let Err(e) = metrics_endpoint::run_metrics_endpoint(&metrics_config).await {
-     tracing::error!("Metrics endpoint error: {e}");
+     tracing::error!(error = %e, "metrics endpoint error");
  }

As per coding guidelines, “Use plain tracing:: macros with structured fields for ordinary logging.” As per path instructions, use “structured tracing fields.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/scout/src/main.rs` around lines 194 - 198, Update the metrics endpoint
logs inside the tokio::spawn closure to use structured tracing fields: emit
metrics_config.address as an address field in the info event and the endpoint
failure as an error field in the error event, while keeping the existing log
messages and behavior.

Sources: Coding guidelines, Path instructions

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/scout/src/cfg/command_line.rs (1)

111-117: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Type the metrics listen address as SocketAddr at the CLI boundary. Both sites treat the metrics address as a bare string, pushing validation from CLI parsing into runtime service startup instead of letting clap/the compiler catch a malformed address immediately.

• crates/scout/src/cfg/command_line.rs#L111-L117: change pub metrics_listen_addr: Option<String> to pub metrics_listen_addr: Option<SocketAddr>; clap derives parsing from SocketAddr's FromStr automatically, giving a clear CLI-level error on malformed input.
• crates/scout/src/main.rs#L179-L182: drop the manual addr_str.parse() / eyre::eyre!("Invalid --metrics-listen-addr...") block and use config.metrics_listen_addr directly as the already-typed SocketAddr.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/scout/src/cfg/command_line.rs` around lines 111 - 117, The metrics
listen address is validated too late as a string. In
crates/scout/src/cfg/command_line.rs:111-117, change CommandLine’s
metrics_listen_addr field to Option<SocketAddr> and ensure the SocketAddr type
is imported; in crates/scout/src/main.rs:179-182, remove the manual
addr_str.parse() and custom invalid-address error handling, passing
config.metrics_listen_addr directly to the metrics/health endpoint startup.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/scout/src/stream.rs (1)

92-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor duplication in the emit-then-map-error pattern.

The emit(ScoutStreamConnection{outcome: Outcome::Error}); ScoutStreamError::X(...) sequence is repeated verbatim across both error paths. A tiny helper (e.g., a closure factory or a record_connection_error fn) could collapse this, though with only two call sites the current inline form is still perfectly readable.

♻️ Optional consolidation

+fn record_connection_error<E: Into<ScoutStreamError>>(e: E) -> ScoutStreamError {
+    emit(ScoutStreamConnection {
+        outcome: Outcome::Error,
+    });
+    e.into()
+}
+
 async fn run_scout_stream_loop(
     machine_id: MachineId,
     options: &Options,
 ) -> Result<(), ScoutStreamError> {
-    let mut client = client::create_forge_client(options).await.map_err(|e| {
-        emit(ScoutStreamConnection {
-            outcome: Outcome::Error,
-        });
-        ScoutStreamError::ClientError(e.to_string())
-    })?;
+    let mut client = client::create_forge_client(options)
+        .await
+        .map_err(|e| record_connection_error(ScoutStreamError::ClientError(e.to_string())))?;

Also applies to: 119-128

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/scout/src/stream.rs` around lines 92 - 97, Optionally consolidate the
repeated connection-error handling in the client creation and subsequent error
paths around create_forge_client and the second map_err block by introducing a
small shared helper or closure that emits ScoutStreamConnection with
Outcome::Error and constructs the corresponding ScoutStreamError. Replace both
inline emit-then-map-error sequences while preserving each path’s existing error
variant and message.

crates/scout/src/metrics.rs (1)

77-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Doc comment undersells the error outcome's coverage.

The comment says error fires only "when the client could not be built," but stream.rs now also emits Outcome::Error on client.scout_stream(...) handshake failures (a separate, later failure point). Worth updating the doc so operators reading this metric's semantics aren't misled about which failures are counted.

✏️ Proposed doc fix

-/// A scout stream connection attempt resolved -- `ok` once the bidirectional
-/// stream is established, `error` when the client could not be built.
+/// A scout stream connection attempt resolved -- `ok` once the bidirectional
+/// stream is established, `error` when the client could not be built or the
+/// stream handshake with the API failed.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/scout/src/metrics.rs` around lines 77 - 90, Update the doc comment for
ScoutStreamConnection to state that the error outcome covers both client
construction failures and scout_stream handshake failures. Keep the existing
description of successful bidirectional stream establishment and the metric
definition unchanged.